expose line function#5
expose line function#5raidancampbell wants to merge 5 commits intoadafruit:masterfrom raidancampbell:patch-1
line function#5Conversation
already exists in framebuf, can be used today by reaching through the oled object with `oled.framebuf.line(16,16,32,32, 1)` changed variable names to conform to conventions Removed trailing whitespace, and attempted to disable `too-many-arguments` linting
tannewt
left a comment
tannewt
left a comment
There was a problem hiding this comment.
One thing to reduce lint clutter.
adafruit_ssd1306.py
Outdated
| """Place text on display""" | ||
| self.framebuf.text(string, xpos, ypos, col) | ||
|
|
||
| #pylint: disable-msg=too-many-arguments |
There was a problem hiding this comment.
I think you only need this disable once if you put it in the method after the comment.
There was a problem hiding this comment.
whoops I meant to re-enable it after this block, I saw some other hunk of code that did that. Copy/pasted a bit too much, fixing...
minor fix for linting hints. re-enable `too-many-arguments` after the block in question
adafruit_ssd1306.py
Outdated
| """Draw a line from initial to final point""" | ||
| self.framebuf.line(xpos0, ypos0, xpos1, ypos1, col) | ||
| #pylint: enable-msg=too-many-arguments | ||
|
|
There was a problem hiding this comment.
I think it would be much more efficient to just add self.line = self.framebuf.line to __init__ — and same for all of the other functions.
|
I'm not sure we want to have to do this in every single driver, and then update those drivers when the |
|
Never thought about that: I agree. I'm unsure how to fully test this however: the lines between I have an open PR at the micropython-adafruit-framebuf repo for mirroring the C logic in python. Ideally I'd like that to be accepted before this gets accepted: I've tested that code as working fine. I haven't ever been able to use the python-backed framebuf: no matter what I try, the actual import is the |
|
@raidancampbell I think the tricky thing here is that the built in framebuf is actually implemented in C and there is a second Python implemented version that can also be converted to bytecode (known as mpy). To test, I'd recommend renaming the python version so that it doesn't conflict and then changing the import in the library. It'd be awesome to get the framebuf module reorganized in CircuitPython so that its in |
|
@raidancampbell Are you hoping to get back to this and test it? |
|
I implemented and tested, but hadn't gotten around to replying. The "cleaner" method of just mapping the functions is a breaking change: the existing wrapper functions specify a default color if none is given. The wrapped function requires the color to be given. If everyone is okay with that, I'll update with the function mapping to replace wrapping. |
# Conflicts: # adafruit_ssd1306.py
- I might have tripped myself up in git here...
| self.framebuf = framebuffer | ||
| # note this is a breaking change: | ||
| # previously these functions were wrapped | ||
| # with a default color of "1" |
There was a problem hiding this comment.
This comment should go in the PR message not the code. I bet col=1 below is actually column not color too.
|
I'm ok with the breaking change. It seems very minor to me. Ping me on Discord is you need git help. |
|
closing to reopen in #9 |
3 participants
already exists in framebuf, can be used today by reaching through the oled object with
oled.framebuf.line(16,16,32,32, 1)